Skip to content

test: added tests for PromQL to match queries to L3 IR - #14

Merged
zzylol merged 4 commits into
mainfrom
add-promql-tests
May 28, 2026
Merged

zzylol merged 4 commits into
mainfrom
add-promql-tests

Conversation

@milindsrivastava1997

@milindsrivastava1997 milindsrivastava1997 commented May 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

End-to-end tests verifying that PromQL queries lower to the correct L3 IR (QueryExpr) trees. Tests are hand-constructed — expected trees are derived from PromQL semantics, not from the lowering implementation — so regressions in the lowering surface as assertion failures.

Test tiers

Tier File What it covers
1 tier1_scan.rs Leaf scans: bare metric, equality / inequality / regex / negative-regex predicates, multi-predicate canonicalisation
2 tier2_agg.rs Cross-series aggregates: sum, avg, count (cardinality), min, max, quantile, stddev, stdvar; single and multiple group keys
3 tier3_range.rs Per-series range functions: rate, increase, count/sum/avg/min/max/quantile/stddev/stdvar_over_time; all lower to Aggregate { child: TimeRange { child: Scan } }
4 tier4_binary.rs Binary ops: arithmetic (+, -, *, /), comparison (>, <, >=, <=); vector matching: on(…), ignoring(…), group_left, group_right
5 tier5_nested.rs Nested pipelines: outer aggregate over rate, outer aggregate over filtered scan, binary op over two independent complex subtrees
6 tier6_schema.rs Output-schema invariants: PromQL scans are closed=false (open); per-series reductions preserve openness; cross-series aggregates freeze to closed=true; binary-op closedness is conjunctive

Skipped / deferred

Test plan

  • cargo test -p asap-e2e — all 44 tests pass across all 6 tiers

@milindsrivastava1997
milindsrivastava1997 marked this pull request as ready for review May 28, 2026 17:09
@zzylol
zzylol merged commit a3b5de8 into main May 28, 2026
1 check passed
@zzylol
zzylol deleted the add-promql-tests branch May 28, 2026 19:08
zzylol added a commit that referenced this pull request Aug 24, 2026
Restructure the flat 19-section developer guide into the three-part
structure the doc owner asked for: Part 1 - Code Architecture, Part 2
- Interfaces and Definitions, Part 3 - How to Add X, Y, Z (each ending
in how to verify). Content is moved, not rewritten:

Part 1 (Mental model first, per doc-owner follow-up, then a new
whole-PR architecture diagram, then "How the current pieces fit
together"):
- old #1 Mental model -> Part 1 #1
- new: whole-PR architecture diagram (TargetSubDAG's two entry points
  through ReplacementStrategy, PlanSpace/cost_sorted, explanation.rs,
  to a downstream consumer) -> Part 1 #2
- old #3 How the current pieces fit together -> Part 1 #3

Part 2:
- old Terminology's "Implementation" definition merged into the
  Glossary as one more entry (### Implementation), next to
  ReplacementStrategy
- old #2 Glossary -> Part 2 #1 (plus the merged Implementation entry
  and old #10 Matcher, retitled to match glossary-entry style)
- old #10 Matcher (implementation.rs) -> ### Matcher inside the
  Glossary; implementation.rs no longer exists, so the stale title
  is fixed
- old #19's definitional content (ReplacementExplanation/
  ExplanationKind shapes, node_hash, why there's no ExplanationRule
  trait, location-text ownership) -> Part 2 #2

Part 3:
- old #4, #5, #6, #7, #13, #14 -> Part 3 #1, Adding a new
  ReplacementStrategy (ending in Testing a new strategy)
- old #8, #9, #15 -> Part 3 #2, Adding or customizing a CostModel
  (ending in Testing a new cost model)
- old #12 -> Part 3 #3, Adding a new sketch algorithm, with its
  stale implementation.rs/binder references fixed to replacement.rs/
  construct_summary vocabulary, plus a new "Verifying a new sketch
  algorithm" close grounded in the existing coverage-matrix tests
- old #11, #16, #17, #18 -> Part 3 #4-#7 (capstone + closing
  reference material); #18's extension-map table's implementation.rs
  row fixed to replacement.rs
- old #19's "Using it"/"Adding a new kind" content -> Part 3 #8,
  Using and extending explanation.rs

cargo build --workspace --all-targets is clean (docs-only change).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
zzylol added a commit that referenced this pull request Aug 26, 2026
…t_graph

Found via manual testing against real corpus queries (not the toy
examples used so far): a single, standalone STDDEV_POP aggregate
crashed dag_export --post-asap with a stack overflow, no sharing or
rewriting involved at all.

Root cause: STDDEV_POP (like AVG) dispatches to
Implementation::PassThrough with no alternative strategy of its own,
so SketchAlgorithmStrategy's only candidate for it is
keep_pre_asap()'s conservative fallback -- Replacement::Summary
wrapping the entire target as SummaryExpr::KeepPreAsap. run_post_asap
was treating this like any other winner and handing it to
export_post_asap, whose find_winner re-checks every node inside a
spliced KeepPreAsap payload by design (so a target nested underneath
one still gets found) -- but this payload structurally is the
enclosing target, so the fresh check finds the identical winner again,
unconditionally, forever.

This is the exact "no-op candidate" concept explanation.rs's own
sketch_finding_reason already excludes from being reported as a
finding. run_post_asap's own winner selection just wasn't applying
that same filter. Fixed by skipping a candidate whose
Replacement::Summary is a bare top-level KeepPreAsap when building
winners, with a regression test exercising the exact repro.

Also verified against several real corpus queries end to end
(previously only exercised with hand-written toy SQL): PR-PKT/FT-PKT
(#44/#49 in synthetic_packet_trace_queries.sql) now correctly show a
genuine Rollup relationship, and P-LEN-AVG (#14) shows the real
avg -> sum/count rewrite -- both screenshotted for the PR description.

Verified: cargo test --workspace, fmt, and clippy all clean.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants